Repository navigation
fix(accuracysnes): measure Mesen2's failing set — F1.10 is 1-vs-3, not 2-vs-2 - #306
Conversation
… 2-vs-2 crossval.sh attributed Mesen2's F1.10 failure to the port-2 limitation while the snes9x block a few lines above said Mesen2 PASSES F1.10. Both could not be true, and nobody had measured it since the catalogue grew, because mesen_crossval.lua reports only an exit code and emu.log does not reach a file this environment can find. mesen_failing_set_probe.lua now writes the set. Read at R_DONE (frame 482) it is exactly F1.03 and F1.10, identical across four runs. The two fail for DIFFERENT reasons, and lumping them under one rationale is what hid it for as long as it did. F1.03 genuinely clocks both ports out of a single latch ($4016 AND $4017), so the port-2 limitation is real for it and only for it. F1.10 reads $4016 only; Mesen2 fails it for the same reason snes9x and ares do -- the automatic read modelled as starting at the vblank edge rather than a few dozen cycles into the line. So F1.10 is 1-vs-3 with RustySNES passing ALONE, and the snes9x block's "Mesen2 delays the start and passes" is retracted. The row is Documented (fullsnes: the read starts ~dot 32.5-95.5 of the first vblank line) and RustySNES passes it only because of a deliberate fix. A first-party accuracy cart being right where three references are wrong is the point of having one, but 1-vs-3 on a scored row is stated rather than left to be mistaken for consensus. One operational gotcha found on the way: the Mesen2 runner can UNDER-report under load. One run returned 1 where every other returned 2, with four other dotnet processes live. --timeout=60 is a wall-clock bound, so a loaded machine can cut the battery short and report a SMALLER failing count -- which reads as "things improved", the most dangerous direction for a gate to be wrong in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WalkthroughThe PR adds a Mesen2 Lua probe for AccuracySNES failing rows. It records measured causes for F1.03 and F1.10, corrects cross-validation results, and documents timeout-related under-reporting under system load. ChangesAccuracySNES verification
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Mesen2
participant Probe as mesen_failing_set_probe.lua
participant Crossval as crossval.sh
participant Docs as AccuracySNES documentation
Mesen2->>Probe: Complete the results menu
Probe->>Mesen2: Read row status bytes
Probe-->>Crossval: Write failing rows and return failure count
Crossval->>Docs: Record F1.03 and F1.10 outcomes
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
Comment |
Antigravity review (Gemini via Ultra)Updates AccuracySNES cross-validation documentation and introduces Blocking issues
Suggestions
Nitpicks
Automated first-pass review by |
There was a problem hiding this comment.
Pull request overview
This PR resolves a documented contradiction in the AccuracySNES cross-validation commentary by adding a concrete Mesen2 measurement of the failing set (not just the failing count), and updates documentation to reflect the measured outcome (F1.10 is a 1-vs-3 divergence).
Changes:
- Add a Mesen2 probe script that dumps the failing set at
R_DONE, enabling stable attribution viaSOURCE_CATALOG.tsv. - Update
crossval.shcommentary to retract the previous “Mesen2 passes F1.10” claim and document the measuredF1.03/F1.10failing set and rationale split. - Update
ares_hostdocumentation and the changelog to reflect the new measured understanding ofF1.10.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/accuracysnes/mesen_failing_set_probe.lua | New Mesen2 Lua probe to write the failing set for offline mapping and attribution. |
| scripts/accuracysnes/crossval.sh | Updates AccuracySNES cross-validation commentary and Mesen2 known-failures rationale based on the measurement. |
| scripts/accuracysnes/ares_host/README.md | Documentation updated to reflect the now-measured F1.10 divergence shape. |
| CHANGELOG.md | Records the correction/retraction and the new measurement-based attribution. |
| local b = rd(STATUS + i) | ||
| -- odd = pass (odd and not 1 = "pass variant"), $FF = skip, even = fail code b/2, 0 = not run | ||
| if b ~= 0xFF and b % 2 == 0 then | ||
| f:write(string.format("%d\t%02X\n", i, b)) | ||
| fail = fail + 1 |
| # MEASURED 2026-08-01 with `scripts/accuracysnes/mesen_failing_set_probe.lua` (read at R_DONE, frame | ||
| # 482): the failing set is exactly `F1.03` and `F1.10`, both code 2. The two fail for DIFFERENT | ||
| # reasons, and lumping them under one rationale is what hid that for as long as it did. |
| measured it since the catalogue grew. `scripts/accuracysnes/mesen_failing_set_probe.lua` now does: | ||
| read at `R_DONE` (frame 482), the failing set is exactly **`F1.03` and `F1.10`**, identical across | ||
| four runs. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/accuracysnes/ares_host/README.md`:
- Around line 56-65: Synchronize the earlier F1.10 table and surrounding summary
with the measured result: identify RustySNES as the sole passing reference, list
snes9x, ares, and Mesen2 as failing, and change the result split to 1-vs-3.
Remove the obsolete port-2 and “doubtful” attribution from the F1.10 prose while
preserving the newer automatic-read timing explanation.
In `@scripts/accuracysnes/mesen_failing_set_probe.lua`:
- Around line 43-56: Update the result handling in the probe’s completion block
to validate the ACSN magic using the same check as mesen_crossval.lua before
reading COUNT or STATUS, and fail closed on invalid or incomplete blocks. In the
status loop, exclude 0x00 from failures because it represents NOTRUN; ensure
incomplete probes are rejected or emit a marker that crossval.sh rejects, while
retaining only nonzero even statuses as failure codes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ebb1348c-8f70-41c1-b650-58dc5f1406ce
📒 Files selected for processing (4)
CHANGELOG.mdscripts/accuracysnes/ares_host/README.mdscripts/accuracysnes/crossval.shscripts/accuracysnes/mesen_failing_set_probe.lua
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: test-light
- GitHub Check: lint
- GitHub Check: accuracysnes
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: review
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Do not commit or vendor the generatedsnesdev_wiki/mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits:<type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors as project releases.
Files:
scripts/accuracysnes/crossval.shCHANGELOG.mdscripts/accuracysnes/ares_host/README.mdscripts/accuracysnes/mesen_failing_set_probe.lua
scripts/accuracysnes/**
⚙️ CodeRabbit configuration file
scripts/accuracysnes/**: The cross-validation harness: the same AccuracySNES image is run on snes9x (through a
libretro host in C) and on Mesen2 (through its test runner and a Lua script), and their
verdicts are compared with the cart's. Its integrity is the whole argument for the
battery, so flag anything that could make a reference appear to agree — a verdict parsed
loosely, a missing-file path that degrades to success, a scene comparison that skips
rather than fails when the golden is absent. A known reference divergence belongs in
SNES9X_KNOWN_FAILURESwith a source citation, never in a widened match.
Files:
scripts/accuracysnes/crossval.shscripts/accuracysnes/ares_host/README.mdscripts/accuracysnes/mesen_failing_set_probe.lua
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Chip-behavior changes must update both the chip implementation and the corresponding
docs/<subsystem>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
CHANGELOG.mdscripts/accuracysnes/ares_host/README.md
CHANGELOG.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
User-visible changes must be recorded under the
[Unreleased]section.For the full pull request diff against its base branch, modify
CHANGELOG.mdwhen user-visible behavior changes, including emulator output, frontend features, CLI flags, public APIs, or AccuracySNES cartridge contents. Do not require it for purely internal changes, tests, comments, or CI configuration.
Files:
CHANGELOG.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).
Files:
CHANGELOG.mdscripts/accuracysnes/ares_host/README.md
🔇 Additional comments (4)
scripts/accuracysnes/mesen_failing_set_probe.lua (2)
1-37: LGTM!Also applies to: 41-42, 49-51, 57-70
38-40: 🗄️ Data Integrity & IntegrationRemove this finding.
crossval.shdoes not invokemesen_failing_set_probe.luaor readMESEN_FAIL_OUT; it invokesmesen_crossval.luaand handles its exit status directly.> Likely an incorrect or invalid review comment.scripts/accuracysnes/crossval.sh (1)
139-144: LGTM!Also applies to: 182-212
CHANGELOG.md (1)
814-839: LGTM!
| **`F1.10` is the interesting one, and it is now measured.** fullsnes says the automatic read begins | ||
| ~dot 32.5–95.5 of the first vblank line rather than at the vblank edge. snes9x fails the row | ||
| (documented instant-latch), ares fails it, and `mesen_failing_set_probe.lua` confirms **Mesen2 fails | ||
| it too** — the failing set read at `R_DONE` is exactly `F1.03` and `F1.10`, identical across four | ||
| runs. So **`F1.10` is 1-vs-3 with RustySNES passing alone**, on a row it passes only because of a | ||
| deliberate fix. | ||
|
|
||
| That is an acceptable place for a first-party accuracy cart to be — being right where the references | ||
| are wrong is the point of having one — but it is stated rather than left to be mistaken for | ||
| consensus. If the fullsnes citation ever turns out to be misread, this row is where it will show. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Synchronize the earlier F1.10 summary with this measured result.
The new paragraph states that RustySNES is the only pass and that Mesen2 fails for automatic-read timing. The table at Lines 37-43 still reports F1.10 as 2-vs-2 with only snes9x failing. Lines 49-54 also retain the obsolete port-2 and “doubtful” attribution.
Update the table to list snes9x and Mesen2 as the other failing references, change the split to 1-vs-3, and remove the obsolete attribution.
As per path instructions, “Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes.”
Proposed documentation update
-| `F1.10` | 2 | snes9x | **2-vs-2** (but see the caveat below) |
+| `F1.10` | 2 | snes9x, Mesen2 | **1-vs-3** — RustySNES alone passes |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/accuracysnes/ares_host/README.md` around lines 56 - 65, Synchronize
the earlier F1.10 table and surrounding summary with the measured result:
identify RustySNES as the sole passing reference, list snes9x, ares, and Mesen2
as failing, and change the result split to 1-vs-3. Remove the obsolete port-2
and “doubtful” attribution from the F1.10 prose while preserving the newer
automatic-read timing explanation.
Source: Path instructions
| if rd(DONE) ~= 0xA5 then return end | ||
|
|
||
| local n = rd16(COUNT) | ||
| local f = io.open(OUT, "w") | ||
| if not f then emu.stop(253) return end | ||
| f:write(string.format("# frames=%d count=%d\n", frames, n)) | ||
| local fail = 0 | ||
| for i = 0, n - 1 do | ||
| local b = rd(STATUS + i) | ||
| -- odd = pass (odd and not 1 = "pass variant"), $FF = skip, even = fail code b/2, 0 = not run | ||
| if b ~= 0xFF and b % 2 == 0 then | ||
| f:write(string.format("%d\t%02X\n", i, b)) | ||
| fail = fail + 1 | ||
| end |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject invalid and incomplete result blocks before reporting a failure set.
At Lines 43-48, R_DONE == 0xA5 is accepted without validating the ACSN magic. scripts/accuracysnes/mesen_crossval.lua performs this validation before reading COUNT and STATUS. Without it, stale or uninitialized WRAM can produce a plausible failure set.
At Lines 52-56, status 0x00 is reported as a failure because it is even and is not 0xFF. The existing decoder classifies 0x00 as NOTRUN, not FAIL. Reject an incomplete probe, or emit a separate marker that crossval.sh rejects. Only nonzero even statuses belong in the failure set.
This invalidates the exact-set claims in scripts/accuracysnes/crossval.sh Lines 182-212 and CHANGELOG.md Lines 814-839 until the probe fails closed.
As per path instructions, scripts/accuracysnes/** is the cross-validation harness, so loose or incomplete verdicts must not produce a reference result.
Proposed fail-closed result handling
if rd(DONE) ~= 0xA5 then return end
+ local magic = string.char(rd(BASE), rd(BASE + 1), rd(BASE + 2), rd(BASE + 3))
+ if magic ~= "ACSN" then
+ emu.log("ACCURACYSNES-BADMAGIC '" .. magic .. "'")
+ emu.stop(253)
+ return
+ end
+
local n = rd16(COUNT)
...
- if b ~= 0xFF and b % 2 == 0 then
+ if b == 0x00 then
+ f:close()
+ emu.stop(252)
+ return
+ elseif b ~= 0xFF and b % 2 == 0 then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if rd(DONE) ~= 0xA5 then return end | |
| local n = rd16(COUNT) | |
| local f = io.open(OUT, "w") | |
| if not f then emu.stop(253) return end | |
| f:write(string.format("# frames=%d count=%d\n", frames, n)) | |
| local fail = 0 | |
| for i = 0, n - 1 do | |
| local b = rd(STATUS + i) | |
| -- odd = pass (odd and not 1 = "pass variant"), $FF = skip, even = fail code b/2, 0 = not run | |
| if b ~= 0xFF and b % 2 == 0 then | |
| f:write(string.format("%d\t%02X\n", i, b)) | |
| fail = fail + 1 | |
| end | |
| if rd(DONE) ~= 0xA5 then return end | |
| local magic = string.char(rd(BASE), rd(BASE + 1), rd(BASE + 2), rd(BASE + 3)) | |
| if magic ~= "ACSN" then | |
| emu.log("ACCURACYSNES-BADMAGIC '" .. magic .. "'") | |
| emu.stop(253) | |
| return | |
| end | |
| local n = rd16(COUNT) | |
| local f = io.open(OUT, "w") | |
| if not f then emu.stop(253) return end | |
| f:write(string.format("# frames=%d count=%d\n", frames, n)) | |
| local fail = 0 | |
| for i = 0, n - 1 do | |
| local b = rd(STATUS + i) | |
| -- odd = pass (odd and not 1 = "pass variant"), $FF = skip, even = fail code b/2, 0 = not run | |
| if b == 0x00 then | |
| f:close() | |
| emu.stop(252) | |
| return | |
| elseif b ~= 0xFF and b % 2 == 0 then | |
| f:write(string.format("%d\t%02X\n", i, b)) | |
| fail = fail + 1 | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/accuracysnes/mesen_failing_set_probe.lua` around lines 43 - 56,
Update the result handling in the probe’s completion block to validate the ACSN
magic using the same check as mesen_crossval.lua before reading COUNT or STATUS,
and fail closed on invalid or incomplete blocks. In the status loop, exclude
0x00 from failures because it represents NOTRUN; ensure incomplete probes are
rejected or emit a marker that crossval.sh rejects, while retaining only nonzero
even statuses as failure codes.
Source: Path instructions
…-3" (#310) E3.06 read TnOUT once at the end of the interval, and TnOUT is a four-bit read-and-clear counter, so the useful range was 8..15 with the wrap one tick above. That is not a band that can be widened -- it is the instrument's ceiling. It now polls timer 2 every ~32 SPC cycles (one timer-2 period), so every read returns 0, 1 or 2 and the running sum cannot lose a tick however long the interval gets. It also asserts the RATIO rather than two absolute counts: |T2 - 8*T0| <= 6, computed on the 65816. Absolute counts pin the poll loop's own cycle cost, which differs between cores for reasons unrelated to the 64 kHz stage -- and that is exactly what made the old row fail on ares while ares' own Timer<128>/Timer<128>/Timer<16> declarations were a correct 8:1. RustySNES and ares now report the identical 6 and 46. ARES_KNOWN_FAILURES drops 5 -> 4. A ceiling in the instrument reads exactly like a defect in the thing measured. RETRACTION: F1.10 is not "1-vs-3 with RustySNES passing alone", and its Mesen2 verdict is phase-fragile. #306 published that reading from a measurement taken at the time. Then rewriting E3.06 -- an UNRELATED APU row -- made Mesen2 PASS F1.10: three identical runs before, three identical runs after. The rewrite changed one uploaded program's length, which moved the cart's execution phase, which moved when F1.10 samples $4212 relative to the vblank edge. So Mesen2's verdict on that row encodes where the cart happens to be, not only what Mesen2 models. snes9x and ares fail it stably; Mesen2 flips. MESEN2_KNOWN_FAILURES drops 2 -> 1 (F1.03 only), and the comment says not to restore an F1.10 entry: either pin the row's sampling point, or accept that Mesen2's verdict on it carries no information. Third instance of the same trap in one session, after E8.01's two rejected drafts and the scene field gate. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
#305 flagged a contradiction in
crossval.shrather than resolving it. This resolves it with a measurement.The contradiction
MESEN2_KNOWN_FAILURESattributed Mesen2'''sF1.10failure to the port-2 limitation. The snes9x block a few lines above said Mesen2 passesF1.10. Both could not be true — and nobody had checked, becausemesen_crossval.luareports only an exit code andemu.logdoes not reach a file this environment can find.The measurement
scripts/accuracysnes/mesen_failing_set_probe.luawrites the failing set. Read atR_DONE(frame 482) it is exactlyF1.03andF1.10, identical across four runs.The two fail for different reasons, and lumping them under one rationale is what hid it:
F1.03genuinely clocks both ports out of a single latch ($4016and$4017), so the port-2 limitation is real for it — and only for it.F1.10reads$4016only. Mesen2 fails it for the same reason snes9x and ares do: the automatic read modelled as starting at the vblank edge rather than a few dozen cycles into the line.So
F1.10is 1-vs-3, with RustySNES passing alonesnes9x, Mesen2 and ares all fail it. The snes9x block'''s "Mesen2 delays the start and passes" is retracted.
The row is Documented — fullsnes puts the read'''s start at ~dot 32.5–95.5 of the first vblank line — and RustySNES passes it only because of a deliberate auto-read-start-timing fix. A first-party accuracy cart being right where three references are wrong is the point of having one, but 1-vs-3 on a scored row is stated rather than left to be mistaken for consensus. If that citation ever turns out to be misread, this row is where it will show.
One operational gotcha found on the way
The Mesen2 runner can under-report under load. One run returned 1 where every other returned 2, with four other
dotnetprocesses live.--timeout=60is a wall-clock bound, so a loaded machine can cut the battery short and report a smaller failing count — which reads as "things improved", the most dangerous direction for a gate to be wrong in. Recorded next to the constant.Verification
mesen_crossval.luadirect: exit 2 on 5/5 runs.REF_PROJ=$PWD/ref-proj bash scripts/accuracysnes/crossval.shon an idle machine —snes9x: OK (14 known),Mesen2: OK (2 known), 54 scenes match on both,2 reference(s) agree with the cart.bash -nclean. Comments, docs, and one new probe script; no behaviour change.🤖 Generated with Claude Code
The change claims that Mesen2 fails AccuracySNES rows
F1.03andF1.10.F1.03fails when both controller ports use one latch.F1.10fails when automatic reading starts at the vblank edge. RustySNES is the only passing reference forF1.10; snes9x, Mesen2, and ares fail.The AccuracySNES dossier updates these assertions and documents Mesen2 runner under-reporting when its 60-second timeout is reached under system load. The coverage denominator does not move.
The claim is false if repeated probes do not reproduce both failures, or if Mesen2 passes either row under the documented test conditions. The under-reporting claim is false if direct runner checks remain complete under system load.